Pull request overview

Updates the specialized HandoffAgentExecutor to reliably produce workflow output in non-streaming mode (addressing the gap that existed compared to AIAgentHostExecutor improvements), and adds unit coverage to prevent regressions.

Changes:

• Emit a final AgentResponse output from HandoffAgentExecutor when TurnToken.EmitEvents is not true (i.e., non-streaming).
• Refactor existing AIAgentHostExecutor unit tests to share common assertions via a new base class.
• Add a new unit test validating HandoffAgentExecutor output behavior across TurnToken.EmitEvents values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

You can also share your feedback on Copilot code review. Take the survey.

Pull request overview

This PR addresses missing agent output in non-streaming Handoff workflows when hosted as an AIAgent, ensuring a final AgentResponse is emitted (vs. only streaming updates), and adds coverage for history persistence.

Changes:

• Update HandoffAgentExecutor to emit an AgentResponse when TurnToken.EmitEvents is not true (treating the flag as “streaming vs. non-streaming” output selection).
• Persist merged outgoing messages into the workflow session chat history for both streaming and non-streaming agent runs.
• Refactor/extend unit tests to cover the expected output type and that outgoing messages are stored in history.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

You can also share your feedback on Copilot code review. Take the survey.

Automated Code Review

Reviewers: 4 | Confidence: 86%

✗ Correctness

This PR refactors chat history management by moving AddMessages and UpdateBookmark calls from per-update (inside WorkflowSession.CreateUpdate and InvokeStageAsync's finally block) to post-completion (in WorkflowHostAgent's RunCoreAsync/RunCoreStreamingAsync). It also adds a new output mode for HandoffAgentExecutor where the full AgentResponse is yielded when streaming updates are not requested. The changes are largely sound, but the streaming path in RunCoreStreamingAsync lacks a try/finally guard, meaning chat history and bookmark updates are silently skipped if the consumer disposes the async enumerator early or cancellation occurs—a behavioral regression from the old finally block in InvokeStageAsync.

✗ Security Reliability

The diff moves chat-history persistence (AddMessages + UpdateBookmark) out of WorkflowSession.InvokeStageAsync (which had a try/finally) into the callers in WorkflowHostAgent. The non-streaming path (RunCoreAsync) is fine because the method is not an async iterator and fully consumes the enumerable. However, RunCoreStreamingAsync is an async iterator (it uses yield return), and the post-loop AddMessages/UpdateBookmark calls are placed after the await foreach loop without a try/finally. In C#, when a consumer disposes an async enumerator early (break, cancellation, exception), code after the yield-return loop is skipped—only pending finally blocks execute. This means on early termination the response messages are never written to history and the bookmark is never advanced, leaving the session in an inconsistent state. This is a reliability regression from the old code whose finally block guaranteed bookmark updates on any exit path.

✗ Test Coverage

The diff moves chat-history bookkeeping out of WorkflowSession.CreateUpdate/InvokeStageAsync and into WorkflowHostAgent.RunCoreAsync and the new RunCoreStreamingAsync path. Two new smoke tests verify that outgoing messages appear in the chat history after RunAsync (non-streaming), and a new HandoffAgentExecutor test covers the emit-events logic. However, there is no corresponding streaming-path test for the chat-history/bookmark change in RunCoreStreamingAsync, which is the most significant behavioural addition in this PR. The removal of the try/finally around UpdateBookmark also changes error-path behaviour that is untested.

✗ Design Approach

The diff moves chat history management from incremental per-update writes (inside WorkflowSession.CreateUpdate) to a deferred batch write after the full turn completes (in WorkflowHostAgent). This is a cleaner design in principle, but the streaming path has a correctness gap: history and bookmark updates only run after the await foreach loop, with no try/finally guard. If a consumer disposes the streaming IAsyncEnumerable early, those commits never execute—yet LastCheckpoint can be advanced mid-stream (via SuperStepCompletedEvent). This creates a divergence between workflow checkpoint state (advanced) and chat history bookmark (stale), potentially causing re-delivery of already-processed messages on the next turn. The old finally block in InvokeStageAsync existed precisely to prevent this. Separately, HandoffAgentExecutor repurposes TurnToken.EmitEvents—documented as controlling whether agent run events are raised—to also mean 'emit a batched full response instead of streaming updates'. The inline comment acknowledges this is a workaround due to a configuration limitation, which is itself a flag for a deeper design issue rather than a fix.

Flagged Issues

• In WorkflowHostAgent.RunCoreStreamingAsync, AddMessages and UpdateBookmark are placed after the await foreach loop with no try/finally. Since this is an async iterator (uses yield return), if the consumer disposes the enumerator early (cancellation, break, or exception), post-loop code is skipped—only pending finally blocks execute. Meanwhile, WorkflowSession.LastCheckpoint can already be advanced mid-stream (by SuperStepCompletedEvent), so skipping UpdateBookmark leaves a divergence: the workflow has moved past a checkpoint but the history bookmark has not, causing re-delivery of already-consumed messages on the next turn. The old finally block in InvokeStageAsync prevented exactly this. A try/finally around the loop is needed to ensure chat history is updated on all exit paths.
• The new smoke tests (Test_SingleAgent_AsAgent_OutgoingMessagesInHistoryAsync, Test_Handoffs_AsAgent_OutgoingMessagesInHistoryAsync) both call RunAsync, exercising only the non-streaming path. The streaming path in RunCoreStreamingAsync contains identical new logic (MessageMerger accumulation, AddMessages, UpdateBookmark) but has zero direct test coverage.

Suggestions

• Add streaming-path equivalents of the two new smoke tests that call RunStreamingAsync, consume all updates, and verify the session's chat history and bookmark are updated afterwards.
• The refactor also changes error-path behaviour: UpdateBookmark was previously in a finally block inside InvokeStageAsync, guaranteeing it ran even when the enumeration threw. It now sits after the foreach loop. Consider adding a test that verifies the intended behaviour when the inner workflow throws—is it intentional that the bookmark is not updated in that case?
• HandoffAgentExecutor repurposes TurnToken.EmitEvents (documented as controlling whether agent run events are raised) to also decide whether to emit a full batched AgentResponse—a distinct concept. This will break callers that set EmitEvents = false to silence streaming events but do not expect a batched response injected into the output stream. A dedicated option on HandoffAgentExecutorOptions that controls batch-vs-streaming output mode would express the intent without overloading an unrelated flag's semantics.

Automated review by chetantoshniwal's agents